fix(ssh): wait for reverse-forward listener before running setup-gpg by skevetter · Pull Request #834 · devsy-org/devsy

skevetter · 2026-07-31T03:40:37Z

Summary

setupGPGAgent (cmd/workspace/ssh.go) started the gpg-agent socket's reverse forward in a background goroutine and immediately ran the remote setup-gpg command, which ends by symlinking the container's gnupg socket to that forward's path (SetupRemoteSocketLink). Nothing synchronized the two — setup-gpg could finish symlinking before the SSH global request that binds the remote listener had actually been acknowledged (client.Listen(...) in pkg/ssh/forward.go), leaving the symlink pointing at a path with no live listener yet.
Splits ReversePortForward into ReverseListen (the blocking bind) and RunReverseForward (the accept loop), mirroring the existing PortForwardWithListener/ForwardOpts pattern already used for the forward-port TOCTOU fix.
Adds startReverseForwardsAndWait, which binds every configured reverse forward before returning, then runs each one's accept loop in the background. setupGPGAgent now calls this and waits for it to return before running setup-gpg, instead of firing the forward off in an unsynchronized goroutine.

Caveat

Reproduced the reported gpg: no gpg-agent running in this session against a real ws3-ssh workspace before and after this change — this fix closes a real race (confirmed via debug logs: the forwarded socket now reliably has a live listener by the time setup-gpg runs, and a real connection does land on it), but it is not sufficient on its own to resolve the reported symptom end-to-end. Further investigation found the container's ~/.gnupg/private-keys-v1.d never receives the secret-key shadow stub files GnuPG needs to recognize an agent-forwarded key as available (ImportGpgKey/configureGPGAgent only ever import the public key + owner-trust). That's a separate, larger gap tracked for follow-up — this PR only fixes the listener race.

Summary by CodeRabbit

Bug Fixes
- Improved reverse SSH port forwarding reliability by binding listeners before forwarding begins.
- GPG-agent forwarding now starts synchronously and reports listener setup failures immediately.
- Forwarding listeners are properly closed when forwarding ends.
- Improved handling of idle timeouts and connection closures during forwarding.

setupGPGAgent started the gpg-agent socket's reverse forward in a background goroutine and immediately ran the remote setup-gpg command, which symlinks the container's gnupg socket to that forward's path. Nothing synchronized the two: setup-gpg could finish symlinking before the SSH global request binding the remote listener had actually been acknowledged, leaving the symlink pointing at a path nothing was listening on yet. Split ReversePortForward into ReverseListen (blocking bind) and RunReverseForward (the accept loop), and add startReverseForwardsAndWait, which binds every configured forward before returning so setup-gpg only runs once the listener is confirmed live. Note: this closes one real race, but is not sufficient on its own to fix "gpg: no gpg-agent running in this session" end to end — the container's ~/.gnupg/private-keys-v1.d never receives the secret-key shadow stubs GnuPG needs to recognize an agent-forwarded key, which is a separate, larger gap in the forwarding setup.

netlify · 2026-07-31T03:40:42Z

✅ Deploy Preview for devsydev canceled.

Name	Link
🔨 Latest commit	`f794851`
🔍 Latest deploy log	https://app.netlify.com/projects/devsydev/deploys/6a6c32c5fd6b1000084d7b61

coderabbitai · 2026-07-31T03:40:45Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a0a00dcf-19d4-4161-9399-2bea9dfb8a7b

📥 Commits

Reviewing files that changed from the base of the PR and between 0ddf01b and f794851.

📒 Files selected for processing (1)

cmd/workspace/ssh.go

🚧 Files skipped from review as they are similar to previous changes (1)

cmd/workspace/ssh.go

📝 Walkthrough

Walkthrough

The change separates reverse listener creation from forwarding, binds workspace listeners before asynchronous startup, and validates GPG-agent forwarding synchronously.

Changes

Reverse forwarding lifecycle

Layer / File(s)	Summary
Reverse forwarding API `pkg/ssh/forward.go`	`ReversePortForward` now uses `ReverseListen` and `RunReverseForward`. `ReverseForwardOpts` carries listener and forwarding settings.
Workspace forwarding setup `cmd/workspace/ssh.go`	Workspace setup binds listeners before forwarding starts. Setup failures close bound listeners. Runtime idle-timeout and EOF exits are suppressed, while unexpected errors are logged. GPG-agent setup returns synchronous forwarding errors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

devsy-org/devsy#763: Both PRs modify SSH forwarding in pkg/ssh/forward.go, including listener lifecycle and forwarding termination behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: waiting for the reverse-forward listener before running setup-gpg.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

netlify · 2026-07-31T03:40:49Z

✅ Deploy Preview for images-devsy-sh canceled.

Name	Link
🔨 Latest commit	`f794851`
🔍 Latest deploy log	https://app.netlify.com/projects/images-devsy-sh/deploys/6a6c32c587b41c0008ba8b4f

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/workspace/ssh.go`:
- Around line 516-537: Update bindReverseForwards to close all previously
created listeners in bound before returning from either a port.ParsePortSpec or
devssh.ReverseListen failure. Reuse the listener references stored in bound,
preserve the existing wrapped errors, and ensure cleanup occurs only for
partial-bind failure paths.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 621024f6-2426-436c-bf81-55c17ca5ba96

📥 Commits

Reviewing files that changed from the base of the PR and between 28c008e and 0ddf01b.

📒 Files selected for processing (2)

cmd/workspace/ssh.go
pkg/ssh/forward.go

…s failure If a later port mapping failed to parse or bind, earlier listeners in bound stayed open on the remote until the SSH transport closed, leaking the bind and breaking a retry against the same address.

skevetter · 2026-07-31T05:29:47Z

Fixed in f794851 — bindReverseForwards now closes any already-bound listeners before returning on a later parse/bind failure.

github-actions Bot added the size/m label Jul 31, 2026

skevetter marked this pull request as ready for review July 31, 2026 04:37

skevetter marked this pull request as draft July 31, 2026 04:38

skevetter marked this pull request as ready for review July 31, 2026 05:20

coderabbitai Bot reviewed Jul 31, 2026

View reviewed changes

Comment thread cmd/workspace/ssh.go

skevetter marked this pull request as draft July 31, 2026 05:26

fix(ssh): close already-bound listeners on partial bindReverseForward…

f794851

…s failure If a later port mapping failed to parse or bind, earlier listeners in bound stayed open on the remote until the SSH transport closed, leaking the bind and breaking a retry against the same address.

skevetter marked this pull request as ready for review July 31, 2026 06:22

skevetter merged commit 988c662 into main Jul 31, 2026
66 checks passed

skevetter deleted the fix/gpg-reverse-forward-race branch July 31, 2026 06:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(ssh): wait for reverse-forward listener before running setup-gpg - #834

fix(ssh): wait for reverse-forward listener before running setup-gpg#834
skevetter merged 2 commits into
mainfrom
fix/gpg-reverse-forward-race

skevetter commented Jul 31, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

netlify Bot commented Jul 31, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

netlify Bot commented Jul 31, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

skevetter commented Jul 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

skevetter commented Jul 31, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Caveat

Summary by CodeRabbit

Uh oh!

netlify Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for devsydev canceled.

Uh oh!

coderabbitai Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

netlify Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for images-devsy-sh canceled.

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

skevetter commented Jul 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

skevetter commented Jul 31, 2026 •

edited by coderabbitai Bot

Loading

netlify Bot commented Jul 31, 2026 •

edited

Loading

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading

netlify Bot commented Jul 31, 2026 •

edited

Loading